Conversation
…vider ad…" This reverts commit 1ad7e10.
dosubot
bot
added
the
size:S
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Now that
kwargsare no longer forwarded intopayloadsfor Anthropic, Gemini, and OpenAI, consider removing or tightening the**kwargsparameter in these methods (or explicitly documenting the accepted options) to avoid confusion and silent no-ops for callers passing extra arguments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that `kwargs` are no longer forwarded into `payloads` for Anthropic, Gemini, and OpenAI, consider removing or tightening the `**kwargs` parameter in these methods (or explicitly documenting the accepted options) to avoid confusion and silent no-ops for callers passing extra arguments.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dosubot
bot
added
the
area:provider
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the inclusion of **kwargs in the API payloads for Anthropic, Gemini, and OpenAI providers. Feedback indicates that this change is a functional regression as it breaks parameter transparency, preventing users from passing provider-specific arguments to the underlying APIs.
| model = model or self.get_model() | ||
|
|
||
| payloads = {**kwargs, "messages": new_messages, "model": model} | ||
| payloads = {"messages": new_messages, "model": model} |
There was a problem hiding this comment.
This change removes the **kwargs from the payloads dictionary. The text_chat method is designed to accept arbitrary keyword arguments (**kwargs) to allow for parameter transparency, enabling users to pass provider-specific parameters directly to the underlying API. Removing **kwargs here prevents this flexibility and can lead to a loss of functionality or extensibility for users who rely on passing additional parameters to the Anthropic API. This reverts the intended behavior of restoring parameter transparency.
payloads = {**kwargs, "messages": new_messages, "model": model}| model = model or self.get_model() | ||
|
|
||
| payloads = {**kwargs, "messages": new_messages, "model": model} | ||
| payloads = {"messages": new_messages, "model": model} |
There was a problem hiding this comment.
Similar to the text_chat method, removing **kwargs from the payloads in text_chat_stream eliminates the ability to pass provider-specific parameters to the Anthropic API. This reduces the flexibility and extensibility of the API, which is a functional regression.
payloads = {**kwargs, "messages": new_messages, "model": model}| model = model or self.get_model() | ||
|
|
||
| payloads = {**kwargs, "messages": context_query, "model": model} | ||
| payloads = {"messages": context_query, "model": model} |
There was a problem hiding this comment.
The removal of **kwargs from the payloads dictionary in text_chat for the Gemini provider restricts the ability to pass through additional, provider-specific parameters. This limits the extensibility of the API and can prevent users from utilizing all available features of the Gemini API that are not explicitly defined in the method signature.
payloads = {**kwargs, "messages": context_query, "model": model}| model = model or self.get_model() | ||
|
|
||
| payloads = {**kwargs, "messages": context_query, "model": model} | ||
| payloads = {"messages": context_query, "model": model} |
There was a problem hiding this comment.
Removing **kwargs from the payloads in text_chat_stream for the Gemini provider removes parameter transparency. This prevents users from passing custom or provider-specific arguments to the underlying API, which is a functional regression and limits the API's flexibility.
payloads = {**kwargs, "messages": context_query, "model": model}| model = model or self.get_model() | ||
| payloads = {**kwargs, "messages": context_query, "model": model} | ||
|
|
||
| payloads = {"messages": context_query, "model": model} |
There was a problem hiding this comment.
The _prepare_chat_payload function is intended to gather all necessary parameters, including **kwargs, for the OpenAI API call. Removing **kwargs from the payloads dictionary here means that any extra parameters passed to the text_chat or text_chat_stream methods will not be forwarded to the OpenAI API. This significantly reduces the flexibility and extensibility of the provider, as users will be unable to leverage additional API features or custom settings supported by OpenAI but not explicitly exposed in the method signature.
payloads = {**kwargs, "messages": context_query, "model": model}1 participant
Reverts #6934
Summary by Sourcery
Enhancements: